Independent Table Elements #34
Conversation
| * | ||
| * @returns A new cursor with updated parameters. | ||
| */ | ||
| query(query: Query) { |
There was a problem hiding this comment.
Hmm I guess one side effect of decoupling table and table cursor is that this now feels out of place 🤔
There was a problem hiding this comment.
Yeah if we want it back - it's going to need to be a function that translates a TableCursor back into a Table call, and that kinda re-adds that dependency this was seeking to remove.
I'm thinking maybe we add this feature into whatever a future QueryBuilder might be, and leave it off for now.
|
|
||
| if (this.next_key) { | ||
| lower_bound = this.next_key | ||
| } | ||
|
|
||
| let indexPosition = this.tableParams.index_position || 'primary' | ||
|
|
||
| if (this.indexPositionField) { |
There was a problem hiding this comment.
I have to admit that I do love the fact that this is no longer needed 😁
src/contract/table-cursor.ts
Outdated
|
|
||
| const rows = |
There was a problem hiding this comment.
This variable assignment does a lot and is a bit difficult to follow so I would probably break it up into different steps.
There was a problem hiding this comment.
I gave this a shot, and added some comments.
contract/src/contract/table-cursor.ts
Lines 85 to 100 in ede31bf
I'm not sure how much more I think we should split it out, unless we wanted to get rid of the ternary operator, and instead do...
let rows = thing
if (decode) {
rows = otherThing
}
| private rowsCount = 0 | ||
| private maxRows: number = Number.MAX_SAFE_INTEGER | ||
|
|
||
| /** |
There was a problem hiding this comment.
I know that these were far from perfect, but we will still want to have some JSDoc statements here, right?
There was a problem hiding this comment.
Yeah absolutely. I just kept seeing so many things changing - that I wanted to redo them all if this was the direction we wanted to go 👍
Co-authored-by: Daniel Fugere <[email protected]>
|
|
||
| const mockClient = makeClient('https://eos.greymass.com') | ||
|
|
||
| suite('Cursor', () => { |
test/tests/table.ts
Outdated
| ) | ||
| }) | ||
| // TODO: Discuss and remove? No longer works since query is a Table method | ||
| // test('should allow you to chain index query statements', async () => { |
There was a problem hiding this comment.
Yeah, I guess we can just remove this. I personally kind of liked the fact that you could chain query statements like this, but given that the cursor and table classes are now built to be used independently, it no longer really makes sense to have a query method on the cursor.
There was a problem hiding this comment.
Removed for now, and left a link to this discussion in the commit.
test/utils/decentium.ts
Outdated
| @@ -0,0 +1,382 @@ | |||
| // generated by @greymass/abi2core | |||
There was a problem hiding this comment.
Did you mean to commit this? 😄
There was a problem hiding this comment.
Yeah the unit tests are using it to test with Struct instances from abi2core.
test/utils/eosio.ts
Outdated
| @@ -0,0 +1,799 @@ | |||
| // generated by @greymass/abi2core | |||
Same logic as before, just different syntax.
Resulting logic is the same, just simplified the Math.min() call
This PR breaks the dependencies between Contract, Table, and TableCursor - which allows developers the opportunity to use each element individually if they choose to do so.